feat(rhai): add apollo.router.operations.rhai.duration histogram metric (copy #9072)#9168
feat(rhai): add apollo.router.operations.rhai.duration histogram metric (copy #9072)#9168mergify[bot] wants to merge 7 commits intodevfrom
Conversation
Emits a new `apollo.router.operations.rhai.duration` (f64 histogram, unit: s) for every Rhai script callback execution across all pipeline stages. Attributes: - `rhai.stage`: RouterRequest, RouterResponse, SupergraphRequest, SupergraphResponse, ExecutionRequest, ExecutionResponse, SubgraphRequest, SubgraphResponse - `rhai.succeeded`: bool — false if the script threw an EvalAltResult - `rhai.is_deferred`: bool — true for @defer stream-chunk callbacks (Supergraph/Execution response stages only; Router-stage deferred path activates automatically when #3642 is resolved) A dedicated RhaiStage enum is defined in rhai/mod.rs rather than reusing PipelineStep from the coprocessor protocol, avoiding semantic coupling to coprocessor-only variants (ConnectorRequest/Response). Uses f64_histogram_with_unit! per dev-docs/metrics.md guidance (coprocessor uses the deprecated f64_histogram! — migration is a separate task). The rhai.succeeded attribute follows the coprocessor convention rather than OTel error.type, intentionally, for symmetry. No separate operation count metric is added; the histogram datapoints are directly countable per attribute combination. Also documents the new metric in standard-instruments.mdx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit 12ed389)
|
@mergify[bot]: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 508dc7cde8f9cfa6a8cab4fb URL: https://www.apollographql.com/docs/deploy-preview/508dc7cde8f9cfa6a8cab4fb
|
So we don't have to do it at every callsite. Also, I removed the `rhai.is_deferred` property from the request side, where it's always `false` and doesn't really make sense. There is no such thing as deferred data on the request side.
goto-bus-stop
left a comment
There was a problem hiding this comment.
I might follow up with some cleanup of the macros here. They don't look necessary and they're confusing to a girl like me
|
|
||
| /// Pipeline stage at which a Rhai script callback was invoked. | ||
| #[derive(Clone, Copy, Debug, Display)] | ||
| enum RhaiStage { |
There was a problem hiding this comment.
This should be replaced with just using PipelineStep (from services/external.rs)
There was a problem hiding this comment.
hmm, maybe. I consider services/external to really be part of the coprocessor plugin (even though it lives in src/services).
I could move PipelineStep elsewhere, like in services/mod.rs, and use it in both places..?
Co-authored-by: Caroline Rodewig <16093297+carodewig@users.noreply.github.com>
Closes #9072
Summary
Design notes
No separate operation count metric: `dev-docs/metrics.md` calls for an `apollo.router.operations.rhai` counter alongside the histogram, following the coprocessor pattern. This PR intentionally omits it — the histogram datapoints are directly countable per attribute combination (`rhai.stage`, `rhai.succeeded`), so a parallel counter would be redundant. Consumers can derive counts from the histogram without the cardinality overhead of a separate instrument.
`rhai.succeeded` attribute: Uses a positive boolean rather than OTel's `error.type` convention, intentionally mirroring the `coprocessor.succeeded` attribute on `apollo.router.operations.coprocessor` for symmetry.
`f64_histogram_with_unit!` vs coprocessor: The coprocessor duration metric uses the deprecated `f64_histogram!` (no unit). This PR uses `f64_histogram_with_unit!` per the guidance in `dev-docs/metrics.md`. Migrating the coprocessor metric is a separate task (would be a breaking rename in Prometheus output).
Test plan
🤖 Generated with Claude Code
This is an automatic copy of pull request #9072 done by Mergify.